Validate all wire messages against the per-version spec JSON schema - #399
Merged
Conversation
commit: |
sync-schema now copies the spec repo's JSON Schema for each version into
src/spec-types/{version}.schema.json (same pinned SHA recorded in SOURCE:
f817239). These are
the inputs for runtime wire-message validation. Vendored verbatim, so they
join schema.ts in .prettierignore.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JHNrpU3Vr6Q1rPZiji7zLY
Every JSON-RPC message the harness sends or receives is now validated (ajv) against the vendored schema.json for the run's spec version, at the wire choke points: sendStatelessRequest (outbound request, JSON response, SSE events), connectStateful (request/result/notifications), and both mock servers (inbound client requests, outbound responses/handler results). Beyond the JSONRPCMessage envelope, messages are validated against the most specific definition the schema has: typed requests/notifications by method const, typed error responses by error.code const (e.g. -32021 -> MissingRequiredClientCapabilityError, the #376 hallucination), and typed results via the XxxRequest -> XxxResult pair, with SEP-2322 InputRequiredResult discriminated on resultType. Violations are recorded per scenario and surfaced by the runners as a synthetic wire-schema-valid check (FAILURE when the implementation under test sent an invalid message) plus a wire-schema-harness-error check when the invalid message was harness-authored. A vitest-wide afterEach guard fails any self-test that recorded violations, so hallucinated fixtures can't ship (#376, #271). Opt-outs are explicit and greppable: per-call skipValidation for deliberately malformed traffic, takeWireViolations() drains in tests that exercise deliberately broken fixtures. Offenders the new validation caught, now fixed: - everything-server stateless path omitted resultType (and the caching hints on server/discover) required by the draft schema - sep-2164-empty-contents fixture omitted resultType/ttlMs/cacheScope (its intended violation, empty contents, is schema-valid prose-only) - connection/mock-server unit-test fixtures used schema-invalid stubs (tools/call without name, ListToolsResult without required members, Tool without inputSchema, ProgressNotification without progressToken, ElicitRequest without message/requestedSchema) - http-standard-headers test mocks returned bare { tools: [] } results - input-required-result-validate-input deliberately sends malformed inputResponses; now marked skipValidation Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JHNrpU3Vr6Q1rPZiji7zLY
Address the review-panel findings on the wire-schema validation PR: - Stateful wire: hook the SDK transport inside connectToServer, wrapping send/onmessage so every raw wire message in both directions is validated for the scenario's spec version -- including the initialize handshake, server->client requests (elicitation/sampling), and the harness's responses to them. Request ids are correlated per direction so responses validate against their typed result definition (e.g. an outbound ElicitResult). The reconstructed stand-ins in connectStateful (fake id: 0 envelopes, Zod-reparsed results) are removed; the hook sees the real bytes. The four scenarios that call connectToServer directly (elicitation-defaults, elicitation-enums, tools, lifecycle) now pass ctx.specVersion and get validation for free; the runner surfaces their violations through the existing recorder. Known remaining gap (documented in the module doc): the client-auth scenarios' bespoke express mock is not instrumented. - skipValidation now skips only the request direction; the implementation's response to a deliberately malformed request is still validated, with a narrow carve-out tolerating the JSON-RPC 2.0 id: null an error response must carry when the request could not be processed (the MCP schema's RequestId forbids null). Covered by new unit tests pinning both the still-validated response and the narrowness of the carve-out. - negative-mrtr.test.ts: the file-wide drain now asserts every recorded violation is implementation-origin (the deliberately broken fixture) and throws on any harness-origin violation. - everything-server: hoist the cacheable-methods set to module scope and replace the per-request res.json monkey-patch with an explicit sendStatelessJson(res, method, payload) helper applied at each dispatch site. - Pin the extracted dispatch maps (methodDefs/errorDefs, e.g. -32021 -> MissingRequiredClientCapabilityError under the draft) per spec version in wire-schema.test.ts, so a schema sync that breaks the const extraction fails loudly instead of degrading to envelope-only validation. Also note the batch branch's requestMethod forwarding limitation. Offender the transport hook flushed out: the everything-server's test_elicitation_sep1034_defaults tool sent a number-typed elicitation field with default: 95.5, which the 2025-11-25 schema forbids (NumberSchema.default is typed integer there; widened to number only in the draft). The fixture and the scenario's expected default are now the integer-valued 95, with comments explaining why. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JHNrpU3Vr6Q1rPZiji7zLY
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JHNrpU3Vr6Q1rPZiji7zLY
Regenerate src/spec-types/*.schema.json via npm run sync-schema at the SHA already recorded in SOURCE. The previously vendored draft schema predated spec PR #3002, so it still required DiscoverResult.serverInfo at the top level; the mock server now correctly reports serverInfo via result _meta, which the stale schema rejected.
pcarleton
force-pushed
the
claude/wire-schema-validation
branch
from
July 27, 2026 11:12
19cfea7 to
4a303f8
Compare
Client --suite runs execute scenarios concurrently over what was a module-global violation recorder, so scenarios could steal or wipe each other's recorded violations. Give each suite scenario its own recorder via AsyncLocalStorage; code outside a scope (single-scenario runs, the sequential server suite, vitest hooks) falls back to the global recorder. Also condense a four-line comment in connection/stateful.ts to three lines.
The released 2025-11-25 schema.json types NumberSchema minimum/maximum/default as integer, contradicting its own schema.ts (default?: number) — a typescript-json-schema artifact, fixed for draft in modelcontextprotocol#2710 and proposed for 2025-11-25 in modelcontextprotocol#3139. Patch the schema at load time so SDK fixtures with fractional number defaults (all tier-1 SDKs send 95.5) are not flagged; restore the sep-1034 scenario and example-server fixture to 95.5. Delete the erratum when #3139 lands and the schemas are re-vendored. Also scope the sequential server-suite loop with withWireRecorder so a connection leaked by a failed scenario cannot attribute late traffic to the next scenario.
pcarleton
approved these changes
Jul 27, 2026
pcarleton
marked this pull request as ready for review
July 27, 2026 14:58
pcarleton
added a commit
that referenced
this pull request
Jul 27, 2026
… 2025-06-18 (#421) Follow-ups from the #399 review: - Stateless mock: validate error replies with a placeholder id so the JSON-RPC-mandated id:null is not recorded as a harness violation, and skip inbound validation when express left the body unparsed (wrong or missing Content-Type) — both were false positives blamed on the wrong party. Same unparsed-body guard in the stateful mock. - Stateful mock: validate client traffic against the protocol version the client declares via MCP-Protocol-Version, falling back to 2025-03-26 when the header is absent (it only exists from 2025-06-18, whose spec says to assume 2025-03-26), instead of always using the run version. - sdk-client: validate batches once as a whole array instead of also per element, which double-recorded every violation. - Make specVersion required on connectToServer/connectStateful/ createServerStateful; add isSpecVersion to types.ts. - Extend the NumberSchema erratum to 2025-06-18 (same generator bug on minimum/maximum, modelcontextprotocol#3139) and add a tripwire test that forces erratum deletion once the schemas are re-vendored. - Document the wire-schema-valid / wire-schema-harness-error check IDs in the README.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requested by Paul Carleton · Slack thread
Before / After
Before: a scenario check could expect a message shape the spec's JSON schema forbids and nothing would catch it. #376 is the motivating example: the sep-2575 capability check required
error.data.requiredCapabilitiesto be the array['sampling'], when the draft schema defines it as aClientCapabilitiesobject ({ "sampling": {} }). Servers had to conform to a hallucinated shape to pass. #271 asks for exactly this kind of schema-derived validation.After: every JSON-RPC message into or out of a scenario — harness requests, implementation responses, SSE events, notifications, server→client elicitation/sampling requests and the harness's responses to them, mock-server traffic — is validated against that spec version's vendored
schema.json. Implementation violations fail the run as a syntheticwire-schema-validcheck; harness-authored violations additionally surface as a loudwire-schema-harness-errorcheck and fail the vitest suite, so a hallucinated fixture or check can no longer ship.How
scripts/sync-schema.tsnow also copiesschema/{version}/schema.jsonintosrc/spec-types/{version}.schema.jsonat the same pinned SHA recorded inSOURCE(71e30695). Vendored verbatim (prettier-ignored) so re-syncs stay clean diffs.src/validation/wire-schema.ts): one lazily compiled ajv instance per spec version (Ajvfor the draft-07 schemas,Ajv2020for 2025-11-25/draft;strict: falsesince the spec schemas aren't authored for ajv strict mode). Messages are checked against the most specific definition available: theJSONRPCMessageenvelope, typed requests/notifications bymethodconst (tools/call→CallToolRequest), typed error responses byerror.codeconst (-32021→MissingRequiredClientCapabilityError— the fix(server-stateless): requiredCapabilities is a ClientCapabilities object, not an array #376 case), and typed results via theXxxRequest→XxxResultpair, with SEP-2322InputRequiredResultdiscriminated onresultType. 2025-03-26 batch arrays are validated per element. A unit test pins the extracted per-version dispatch maps (methodDefs/errorDefs), so a future schema sync that breaks the const extraction fails loudly instead of silently degrading to envelope-only validation.sendStatelessRequest(outbound request, JSON response body, every SSE event).connectToServerwraps the SDK transport'ssend/onmessage, so every raw wire message in both directions is validated for the scenario's spec version — including theinitializehandshake, server→client requests (elicitation/sampling viaclient.setRequestHandler), and the harness's responses to them. Request ids are correlated per direction so each response is validated against its request's typed result definition (e.g. an outboundElicitResult). No reconstructed stand-ins: the hook sees the real bytes, and scenarios that use the SDKClientdirectly (elicitation-defaults, elicitation-enums, tools, lifecycle) are covered with no per-scenario code.createServerStateless(inbound client requests, every outbound response),createServerStateful(inbound client requests, handler results).src/runner/{server,client}.tsappend the synthetic checks after each scenario.afterEach(src/validation/vitest-hooks.ts) fails any test during which a violation was recorded — in self-tests both sides of the wire are harness-authored, so this is the anti-hallucination backstop.src/validation/wire-schema.test.ts): the exact fix(server-stateless): requiredCapabilities is a ClientCapabilities object, not an array #376 shape is asserted to FAIL under the draft schema and to produce a failingwire-schema-validcheck when fed through the choke point; a harness-built invalidtools/callis asserted to produce thewire-schema-harness-errorhard-failure path; the fixedClientCapabilities-object shape is asserted to pass.skipValidationper call onsendStatelessRequestskips the request direction only, for scenarios that intentionally send malformed traffic (used byinput-required-result-validate-input); the implementation's response is still validated, with one narrow carve-out tolerating the JSON-RPC 2.0id: nullan error response must carry when the request could not be processed (the MCP schema'sRequestIdforbids null).takeWireViolations()drains in tests that intentionally exercise nonconformant traffic (negative.test.tssep-2549, the proxy-body tests inconnection.test.ts/stateless.test.ts, and the missing-_metarejection tests inmock-server.test.ts); thenegative-mrtr.test.tsdrain additionally asserts every recorded violation is implementation-origin (the deliberately broken fixture) and throws on any harness-origin violation.Offenders the validation caught (and fixes)
examples/servers/typescript/everything-server.ts— stateless (draft) path omitted theresultTypethe draftResultdefinition requires on every result, and the caching hints onserver/discover(DiscoverResultrequiresttlMs/cacheScope). Fixed via an explicitsendStatelessJson(res, method, payload)helper applied at each dispatch site (module-scope cacheable-methods set; nores.jsonmonkey-patching).examples/servers/typescript/everything-server.ts(caught by the stateful transport hook) —test_elicitation_sep1034_defaultssends anelicitation/createwithdefault: 95.5on a number-typed field, which the released 2025-11-25 schema.json rejects: it typesNumberSchema.defaultasinteger, a generator artifact contradicting its own schema.ts (fixed for draft in modelcontextprotocol#2710; proposed for 2025-11-25 in modelcontextprotocol#3139). Every tier 1 SDK fixture sends the same fractional default, so the validator patches the loaded 2025-11-25 schema with a documented erratum (applySchemaErratainsrc/validation/wire-schema.ts) rather than forcing coordinated fixture changes; the erratum is deleted when #3139 lands and the schemas are re-vendored.examples/servers/typescript/sep-2164-empty-contents.ts— omittedresultType/ttlMs/cacheScopeincidentally; its intended violation (emptycontents) is schema-valid, prose-forbidden. Fixed so the fixture is invalid only in the way it exists to test.src/scenarios/server/input-required-result.ts(input-required-result-validate-input) — deliberately sendsinputResponses: 12345/null; now opts out per call viaskipValidation(request direction only).src/scenarios/server/http-standard-headers.test.ts— mock servers returned bare{ tools: [...] }results missing the draft-required members; fixtures made schema-valid.src/connection/connection.test.ts/stateless.test.ts— stub responses likeresult: { ok: true }fortools/call,notifications/progresswithoutprogressToken,elicitation/createwith empty params, and harnesstools/callrequests withoutparams.name; all corrected to spec-valid shapes.src/mock-server/mock-server.test.ts— handler fixtures returnedTools without the requiredinputSchemaand atools/callwithoutparams.name; corrected. (createServerForidentity assertion replaced with a behavioral check since the factory now binds the spec version.)sep-2549-no-caching-hints.ts(missing hints is the point) andsep-2322-mrtr-broken-server.ts(omittingresultTypeis the point) — their tests drain the recorder explicitly (implementation-origin only).Known gaps / follow-ups
initialize,sse-retry,elicitation-defaults,http-baseand its subclasses) and server scenarios using rawfetchor their own SDKClient(sse-polling,sse-multiple-streams,http-standard-headers,lifecycle,stateless) record no wire traffic. Full inventory and suggested direction tracked in Wire-schema validation does not see raw-HTTP / inline-mock scenario traffic #418.requestMethodto every batch element; no caller sends batched requests today (noted inline).Refs #376, #271.
🤖 Generated with Claude Code
https://claude.ai/code/session_01JHNrpU3Vr6Q1rPZiji7zLY
Generated by Claude Code